From: Jan Beulich <jbeulich@suse.com>
Date: Tue, 5 Jan 2021 12:20:13 +0000 (+0100)
Subject: vPCI/MSI-X: tidy init_msix()
X-Git-Tag: archive/raspbian/4.16.0+51-g0941d6cb-1+rpi1~2^2~42^2~1204
X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https://%22%22/%22http:/www.example.com/cgi/%22https:/%22%22?a=commitdiff_plain;h=6fbb41258e9f733cd2b9254efae035c350d93920;p=xen.git

vPCI/MSI-X: tidy init_msix()

First of all introduce a local variable for the to be allocated struct.
The compiler can't CSE all the occurrences (I'm observing 80 bytes of
code saved with gcc 10). Additionally, while the caller can cope and
there was no memory leak, globally "announce" the struct only once done
initializing it. This also removes the dependency of the function on
the caller cleaning up after it in case of an error.

Also prefer a local variable over using a structure field previously
set from this very variable.

Finally move the call to vpci_add_register() ahead of all further
initialization of the struct, to bail early in case of error.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---

diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index e008f92c1c..846a048a4a 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -442,6 +442,7 @@ static int init_msix(struct pci_dev *pdev)
     uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
     unsigned int msix_offset, i, max_entries;
     uint16_t control;
+    struct vpci_msix *msix;
     int rc;
 
     msix_offset = pci_find_cap_offset(pdev->seg, pdev->bus, slot, func,
@@ -453,34 +454,37 @@ static int init_msix(struct pci_dev *pdev)
 
     max_entries = msix_table_size(control);
 
-    pdev->vpci->msix = xzalloc_flex_struct(struct vpci_msix, entries,
-                                           max_entries);
-    if ( !pdev->vpci->msix )
+    msix = xzalloc_flex_struct(struct vpci_msix, entries, max_entries);
+    if ( !msix )
         return -ENOMEM;
 
-    pdev->vpci->msix->max_entries = max_entries;
-    pdev->vpci->msix->pdev = pdev;
+    rc = vpci_add_register(pdev->vpci, control_read, control_write,
+                           msix_control_reg(msix_offset), 2, msix);
+    if ( rc )
+    {
+        xfree(msix);
+        return rc;
+    }
+
+    msix->max_entries = max_entries;
+    msix->pdev = pdev;
 
-    pdev->vpci->msix->tables[VPCI_MSIX_TABLE] =
+    msix->tables[VPCI_MSIX_TABLE] =
         pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
-    pdev->vpci->msix->tables[VPCI_MSIX_PBA] =
+    msix->tables[VPCI_MSIX_PBA] =
         pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
 
-    for ( i = 0; i < pdev->vpci->msix->max_entries; i++)
+    for ( i = 0; i < max_entries; i++)
     {
-        pdev->vpci->msix->entries[i].masked = true;
-        vpci_msix_arch_init_entry(&pdev->vpci->msix->entries[i]);
+        msix->entries[i].masked = true;
+        vpci_msix_arch_init_entry(&msix->entries[i]);
     }
 
-    rc = vpci_add_register(pdev->vpci, control_read, control_write,
-                           msix_control_reg(msix_offset), 2, pdev->vpci->msix);
-    if ( rc )
-        return rc;
-
     if ( list_empty(&d->arch.hvm.msix_tables) )
         register_mmio_handler(d, &vpci_msix_table_ops);
 
-    list_add(&pdev->vpci->msix->next, &d->arch.hvm.msix_tables);
+    pdev->vpci->msix = msix;
+    list_add(&msix->next, &d->arch.hvm.msix_tables);
 
     return 0;
 }